Skip to content

Allow global_asm! in statement positions#156582

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
daxpedda:global-asm-statement
May 18, 2026
Merged

Allow global_asm! in statement positions#156582
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
daxpedda:global-asm-statement

Conversation

@daxpedda
Copy link
Copy Markdown
Contributor

@daxpedda daxpedda commented May 14, 2026

View all comments

This PR makes it possible to put global_asm! in statement positions. This is particularly useful for proc-macros, where you otherwise have to wrap them in mod foo { global_asm!(...); }.

I'm happy to open an ACP first (or a design meeting?).
I would also assume this needs sign-off from the lang-team?

Previously discussed on Zulip.

r? @Amanieu

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 14, 2026
@rust-log-analyzer

This comment has been minimized.

@daxpedda daxpedda marked this pull request as draft May 14, 2026 18:47
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 14, 2026
@daxpedda daxpedda force-pushed the global-asm-statement branch from fc5f2bf to 76f8edb Compare May 14, 2026 21:34
Comment thread compiler/rustc_expand/src/base.rs
@daxpedda daxpedda force-pushed the global-asm-statement branch from 76f8edb to d0629e2 Compare May 14, 2026 22:14
@daxpedda daxpedda marked this pull request as ready for review May 14, 2026 22:15
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 14, 2026
@Amanieu
Copy link
Copy Markdown
Member

Amanieu commented May 15, 2026

I'm happy with the asm part of this but I'm not an expert on the macro infrastructure, so...

r? @petrochenkov

@rustbot rustbot assigned petrochenkov and unassigned Amanieu May 15, 2026
Comment thread compiler/rustc_expand/src/base.rs Outdated
Comment thread compiler/rustc_expand/src/base.rs Outdated
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 18, 2026
@daxpedda daxpedda requested a review from petrochenkov May 18, 2026 13:50
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 18, 2026
Comment thread compiler/rustc_builtin_macros/src/asm.rs Outdated
Comment thread compiler/rustc_builtin_macros/src/asm.rs Outdated
@petrochenkov
Copy link
Copy Markdown
Contributor

Thanks!
r=me after addressing the remaining comments and squashing commits.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 18, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 18, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@daxpedda daxpedda force-pushed the global-asm-statement branch from 3e29858 to 450cdb5 Compare May 18, 2026 14:03
@daxpedda
Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 18, 2026
@daxpedda daxpedda requested a review from petrochenkov May 18, 2026 14:04
@daxpedda

This comment was marked as resolved.

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 18, 2026

@daxpedda: 🔑 Insufficient privileges: not in review users

@daxpedda

This comment was marked as resolved.

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 18, 2026

@daxpedda: 🔑 Insufficient privileges: not in review users

@daxpedda

This comment was marked as resolved.

@petrochenkov
Copy link
Copy Markdown
Contributor

@bors r+

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 18, 2026

📌 Commit 450cdb5 has been approved by petrochenkov

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 18, 2026
rust-bors Bot pushed a commit that referenced this pull request May 18, 2026
…uwer

Rollup of 13 pull requests

Successful merges:

 - #156709 (stdarch subtree update)
 - #155006 (stabilize `feature(cfg_target_has_atomic_equal_alignment)`)
 - #156444 (Implement `OsStr::split_at`)
 - #156582 (Allow `global_asm!` in statement positions)
 - #156661 (Remove `UncheckedIterator`)
 - #152367 (Derives `Copy` for `ffi::FromBytesUntilNulError`)
 - #156443 (Fix invalid suggestion for parenthesized break)
 - #156606 (Add pext/pdep as aliases for extract_bits/deposit_bits)
 - #156630 (Replace `println!` with `assert!` in HashMap documentation examples)
 - #156644 (Widen the result of `widening_mul`.)
 - #156653 (Update `sysinfo` version to `0.39.2`)
 - #156697 (Add diagnostic items for IoBufReader and StdinLock)
 - #156704 (reduce usage of `box_patterns` in tests)
@rust-bors rust-bors Bot merged commit 59eda84 into rust-lang:main May 18, 2026
11 checks passed
@rustbot rustbot added this to the 1.97.0 milestone May 18, 2026
rust-timer added a commit that referenced this pull request May 18, 2026
Rollup merge of #156582 - daxpedda:global-asm-statement, r=petrochenkov

Allow `global_asm!` in statement positions

This PR makes it possible to put `global_asm!` in statement positions. This is particularly useful for proc-macros, where you otherwise have to wrap them in `mod foo { global_asm!(...); }`.

I'm happy to open an ACP first (or a design meeting?).
I would also assume this needs sign-off from the lang-team?

Previously discussed on [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/216763-project-inline-asm/topic/Item.20position.20global_asm/with/581784943).

r? @Amanieu
@folkertdev
Copy link
Copy Markdown
Contributor

Does this not need some sort of FCP? Currently this extension is effectively instantly stable, and as #156760 shows I think there are some places in the compiler where we just assume that global asm is top-level.

@RalfJung
Copy link
Copy Markdown
Member

Yeah let's revert this so that the lang team can take a look without time pressure.

JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request May 24, 2026
…-item, r=workingjubilee

Revert "Allow `global_asm!` in statement positions"

This reverts commit 450cdb5.

Based on discussion in [#t-lang > insta-stable &rust-lang#96;global_asm!&rust-lang#96; as inner items](https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/insta-stable.20.60global_asm.21.60.20as.20inner.20items/with/597411711), rust-lang#156582 should not have been merged without FCP.

We've also since found a bug in the implementation (or really, an assumption in the existing code that the PR invalidates), so to re-land this

- new behavior should be behind a feature gate
- the fix in rust-lang#156855 (comment) should be included (and reviewed by someone familiar with `rustc_resolve`)

Then we keep it unstable for a bit, and see if the fuzzer finds anything else. Nothing here is particularly controversial I think, but we need to do our due diligence.

r? @ghost
rust-timer added a commit that referenced this pull request May 24, 2026
Rollup merge of #156884 - folkertdev:revert-global-asm-inner-item, r=workingjubilee

Revert "Allow `global_asm!` in statement positions"

This reverts commit 450cdb5.

Based on discussion in [#t-lang > insta-stable `global_asm!` as inner items](https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/insta-stable.20.60global_asm.21.60.20as.20inner.20items/with/597411711), #156582 should not have been merged without FCP.

We've also since found a bug in the implementation (or really, an assumption in the existing code that the PR invalidates), so to re-land this

- new behavior should be behind a feature gate
- the fix in #156855 (comment) should be included (and reviewed by someone familiar with `rustc_resolve`)

Then we keep it unstable for a bit, and see if the fuzzer finds anything else. Nothing here is particularly controversial I think, but we need to do our due diligence.

r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants